Skip to content

theta union get_result() fix#357

Merged
AlexanderSaydakov merged 1 commit into
masterfrom
theta_union_fix
Apr 12, 2023
Merged

theta union get_result() fix#357
AlexanderSaydakov merged 1 commit into
masterfrom
theta_union_fix

Conversation

@AlexanderSaydakov

Copy link
Copy Markdown
Contributor

fixes #356

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 4683376611

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.016%

Totals Coverage Status
Change from base Build 4631768738: 0.0%
Covered Lines: 5072
Relevant Lines: 5228

💛 - Coveralls

@AlexanderSaydakov

Copy link
Copy Markdown
Contributor Author

The essence of the fix is line 68: if(union_theta_ >= table_.theta_) as opposed to if(union_theta_ >= theta).
The rest is for clarity (slightly rearranged flow in get_result) and efficiency (std::min instead of conditions).

@jmalkin jmalkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AlexanderSaydakov
AlexanderSaydakov merged commit 034749f into master Apr 12, 2023
@AlexanderSaydakov
AlexanderSaydakov deleted the theta_union_fix branch April 12, 2023 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

the theta_union estimate value will change dramatically with the order of merge

3 participants